Skip to content

[Repo Assist] fix: ProvidedTypeDefinition.Logger broken (new ref each call); add 5 tests for warning + enum round-trip#501

Merged
dsyme merged 2 commits intomasterfrom
repo-assist/test-improvements-logger-enum-20260411-5b12cad7f2310ece
Apr 13, 2026
Merged

[Repo Assist] fix: ProvidedTypeDefinition.Logger broken (new ref each call); add 5 tests for warning + enum round-trip#501
dsyme merged 2 commits intomasterfrom
repo-assist/test-improvements-logger-enum-20260411-5b12cad7f2310ece

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

🤖 This PR was created by Repo Assist, an automated AI assistant.

Summary

This PR bundles a bug fix and new tests (Task 9: Testing Improvements).

Bug fix: ProvidedTypeDefinition.Logger was silently broken

The static Logger member was implemented as a computed property:

// Before — creates a fresh ref cell on every access; impossible to set
static member Logger: (string -> unit) option ref = ref None

Every read of ProvidedTypeDefinition.Logger returned a different ref object, so calling ProvidedTypeDefinition.Logger := Some f would write into a temporary cell that is immediately discarded. The Logger was effectively inoperative: the "all static parameters optional" warning (added in 8.3.0 / PR #428) could never fire via the Logger, and type-creation trace messages were silently dropped.

Fix: introduces a static let loggerRef field and makes Logger return it:

static let loggerRef: (string -> unit) option ref = ref None
...
static member Logger: (string -> unit) option ref = loggerRef

New tests (5 tests, 131 total)

BasicErasedProvisionTests.fs — 3 new tests for the all-optional warning:

  • DefineStaticParameters warns when all static parameters have defaults — regression for the warning path (which was previously unreachable due to the Logger bug)
  • DefineStaticParameters does not warn when at least one parameter has no default
  • DefineStaticParameters does not warn when there are no static parameters

GenerativeEnumsProvisionTests.fs — 2 new tests for non-Int32 enum round-trip via target context:

  • Byte enum underlying type is correct when read via target context (ReadRelatedAssembly) — exercises TargetTypeDefinition.GetEnumUnderlyingType() for a byte-backed enum
  • Int64 enum underlying type is correct when read via target context (ReadRelatedAssembly) — same for int64-backed enums

These complement the existing runtime-path enum tests (Assembly.Load) by covering the design-time IL-reader path, which was fixed in PR #470/#475.

Trade-offs

  • The Logger ref is now module-level shared state. Tests that manipulate it should save/restore the previous value (all three new tests do this via try/finally).

Test Status

✅ All 131 tests pass (dotnet test tests/FSharp.TypeProviders.SDK.Tests.fsproj -c Release)

Generated by 🌈 Repo Assist, see workflow run. Learn more.

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@7ee2b60744abf71b985bead4599640f165edcd93

…add tests for all-optional warning and non-Int32 enum target-context round-trip

The Logger static member was implemented as a computed property returning
`ref None` on every access, making it impossible to set. Changed to a
`static let loggerRef` field and made Logger return that shared ref.

New tests added (Task 9):
- 3 tests for the all-optional static parameters warning via ProvidedTypeDefinition.Logger
- 2 tests that read non-Int32 enums back via TargetContext.ReadRelatedAssembly,
  exercising the TargetTypeDefinition.GetEnumUnderlyingType() path

All 131 tests pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@dsyme dsyme marked this pull request as ready for review April 13, 2026 12:01
@dsyme dsyme merged commit c843603 into master Apr 13, 2026
2 checks passed
dsyme pushed a commit that referenced this pull request Apr 17, 2026
….6.0 (#504)

🤖 *This is an automated PR from Repo Assist.*

## Summary

**Task 9 – Testing Improvements**: Add `GenerativePropertiesTests.fs`
with 5 new property tests for generative types.

**Task 10 – Take the Repository Forward**: Update `RELEASE_NOTES.md` to
prepare release 8.6.0.

---

## Changes

### New: `tests/GenerativePropertiesTests.fs`

Five tests covering properties in generative type providers:

1. **Instance read-only property** – presence, type,
`CanRead`/`CanWrite`, non-static getter
2. **Instance read-write property** – getter and setter present, correct
method names
3. **Static read-only property** – presence, static getter
4. **Name-lookup for all properties** – calls `GetProperty` by name for
each of the 4 properties on the generated type; directly exercises the
`ILPropertyDefs` lazy `FindByName` dictionary added in #502
5. **Property count** – verifies the correct number of properties is
emitted

### Updated: `RELEASE_NOTES.md`

Added 8.6.0 entry documenting:
- Bug fix: `ProvidedTypeDefinition.Logger` creating a new delegate
reference on each call (#501)
- Refactor/Performance: `ILFieldDefs`/`ILEventDefs`/`ILPropertyDefs`
concrete classes with lazy O(1) name-lookup caches (#502)
- These new tests

---

## Test Status

✅ **136/136 tests pass** (baseline was 131; 5 new tests added, all
green)

```
Passed!  - Failed: 0, Passed: 136, Skipped: 0, Total: 136
```




> Generated by 🌈 Repo Assist, see [workflow
run](https://github.com/fsprojects/FSharp.TypeProviders.SDK/actions/runs/24430249769).
[Learn
more](https://github.com/githubnext/agentics/blob/main/docs/repo-assist.md).
>
> To install this [agentic
workflow](https://github.com/githubnext/agentics/blob/97143ac59cb3a13ef2a77581f929f06719c7402a/workflows/repo-assist.md),
run
> ```
> gh aw add
githubnext/agentics@97143ac
> ```

<!-- gh-aw-agentic-workflow: Repo Assist, engine: copilot, model: auto,
id: 24430249769, workflow_id: repo-assist, run:
https://github.com/fsprojects/FSharp.TypeProviders.SDK/actions/runs/24430249769
-->

<!-- gh-aw-workflow-id: repo-assist -->

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
dsyme pushed a commit that referenced this pull request Apr 20, 2026
…ase 8.7.0 (#506)

🤖 *This is an automated pull request from Repo Assist.*

## Summary

Adds `GenerativeCustomAttributeTests.fs` — 5 regression tests for custom
attribute encoding in the generative IL writer. These cover attribute
targets and argument types not previously exercised by the existing
tests in `BasicGenerativeProvisionTests.fs` (which only tested
attributes on *properties*).

Also updates `RELEASE_NOTES.md` with a new `8.7.0` entry covering this
and the previously-merged `GenerativeMethodsTests` (#505).

## New tests

| Test | What it verifies |
|------|-----------------|
| `Custom attribute on a generative type round-trips correctly` |
`ObsoleteAttribute(string, bool)` placed on the *type definition itself*
|
| `Custom attribute with bool constructor argument round-trips
correctly` | `bool` values survive the ECMA-335 encode/decode path |
| `Custom attribute with enum constructor argument round-trips
correctly` | `DebuggerBrowsableAttribute(DebuggerBrowsableState.Never)`
— enum arg decoded as underlying int |
| `Multiple custom attributes on a single generative method are all
preserved` | `defineCustomAttrs` writes **all** attributes, not just the
first |
| `Custom attribute on a generative method has correct string argument`
| `DescriptionAttribute(string)` on an instance method |

## Root cause / motivation

Several custom-attribute encoding bugs were fixed in earlier PRs
(`#432`, `#490`, `#501`). This PR adds focused regression tests for the
remaining un-covered paths:
- attributes on **types** (not just members)  
- attributes on **methods** (not just properties)  
- **bool** and **enum** constructor argument encoding  
- multiple attributes on a single member

## Test Status

```
Passed! - Failed: 0, Passed: 147, Skipped: 0, Total: 147, Duration: 7 s
```
All 147 tests pass (142 pre-existing + 5 new).




> Generated by 🌈 Repo Assist, see [workflow
run](https://github.com/fsprojects/FSharp.TypeProviders.SDK/actions/runs/24617397828).
[Learn
more](https://github.com/githubnext/agentics/blob/main/docs/repo-assist.md).
>
> To install this [agentic
workflow](https://github.com/githubnext/agentics/blob/97143ac59cb3a13ef2a77581f929f06719c7402a/workflows/repo-assist.md),
run
> ```
> gh aw add
githubnext/agentics@97143ac
> ```

<!-- gh-aw-agentic-workflow: Repo Assist, engine: copilot, model: auto,
id: 24617397828, workflow_id: repo-assist, run:
https://github.com/fsprojects/FSharp.TypeProviders.SDK/actions/runs/24617397828
-->

<!-- gh-aw-workflow-id: repo-assist -->

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant